add more logs and support for tcp keepalive #1992
Conversation
|
✅ Docker image ready for
Use this tag to pull the image for testing. 📋 Copy commandsgcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/robusta-runner:a789e8f
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/robusta-runner:a789e8f me-west1-docker.pkg.dev/robusta-development/development/robusta-runner-dev:a789e8f
docker push me-west1-docker.pkg.dev/robusta-development/development/robusta-runner-dev:a789e8fPatch Helm values in one line: helm upgrade --install robusta robusta/robusta \
--reuse-values \
--set runner.image=me-west1-docker.pkg.dev/robusta-development/development/robusta-runner-dev:a789e8f |
WalkthroughAdds four TCP keepalive environment constants, exposes them in env vars, applies keepalive socket options in the WebSocket receiver when enabled, adds debug logging around WebSocket ping/keepalive and action/stream handling, and updates Slack tests to fetch messages by timestamp. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/utils/slack_utils.py`:
- Around line 26-36: The return type annotation of get_message_by_ts is
incorrect because it can return None; update the signature to return
Optional[str] and add the corresponding import from typing (e.g., from typing
import Optional), keeping the existing logic that uses
self.client.conversations_history and messages; ensure the annotation reads
Optional[str] for get_message_by_ts(self, ts: str) -> Optional[str].
🧹 Nitpick comments (2)
tests/test_slack.py (2)
26-27: Good improvement using timestamp-based retrieval for deterministic testing.This avoids race conditions when tests share a channel. Consider adding an explicit None check for clearer failure messages if the message isn't found:
ts = slack_sender.send_finding_to_slack(finding, slack_params, False) message = slack_channel.get_message_by_ts(ts) assert message is not None, f"Message with ts={ts} not found" assert message == msg
77-80: Consider updating other tests to useget_message_by_ts()for consistency.This test and others (
test_send_file_named_tempfile_fails,test_temporary_file_creation_failure) still useget_latest_message(). If the rationale for the change intest_send_to_slackapplies here, these could be updated similarly by capturing thetsreturn value. This would align with the guidance inslack_utils.pyto prefer timestamp-based retrieval.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/test_slack.pytests/utils/slack_utils.py
🧰 Additional context used
🧬 Code graph analysis (1)
tests/test_slack.py (2)
src/robusta/integrations/slack/sender.py (1)
send_finding_to_slack(660-682)tests/utils/slack_utils.py (1)
get_message_by_ts(26-36)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: run_tests
- GitHub Check: run_tests
🔇 Additional comments (1)
tests/utils/slack_utils.py (1)
20-24: Helpful comment for guiding users to the better API.The note clarifies when to prefer timestamp-based retrieval over latest message retrieval.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
No description provided.